Redirect order-status targets to customer account instead of checkout targets #4410

lihaokx · 2024-09-03T14:31:06Z

WHY are these changes introduced?

Fixes: https://github.com/Shopify/core-issues/issues/74362

WHAT is this pull request doing?

4 targets starting by customer-account.order-status were treated as checkout targets. Since, they are on customer acocunt-web, we will treat them as customer account targets instead of checkout targets

Tophat

You can see this video of 🎩 https://videobin.shopify.io/v/Bm4kBg
Or try this cli console link: https://handy-delay-carried-oxide.trycloudflare.com/extensions/dev-console After clicking order-status target, you should be redirected to customer account

Measuring impact

How do we know this change was effective? Please choose one:

n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
Existing analytics will cater for this addition
PR includes analytics changes to measure impact

Checklist

I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've considered possible documentation changes

github-actions · 2024-09-03T14:31:24Z

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

github-actions · 2024-09-03T14:35:19Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	72.86% (+0.09% 🔼)	8269/11349
🟡	Branches	69.45% (+0.04% 🔼)	4036/5811
🟡	Functions	71.61% (+0.03% 🔼)	2157/3012
🟡	Lines	73.21% (+0.1% 🔼)	7822/10685

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / utilities.ts	82.35% (-2.65% 🔻)	80% (+3.08% 🔼)	100%	85.71% (-2.52% 🔻)
🟢	`...` / ConcurrentOutput.tsx	98.39% (-1.61% 🔻)	90.91% (-4.55% 🔻)	100%	98.33% (-1.67% 🔻)

Test suite run success

1861 tests passing in 845 suites.

Report generated by 🧪jest coverage report action from e34f232

lihaokx · 2024-09-03T18:58:47Z

packages/app/src/cli/services/dev/extension/utilities.ts

@@ -41,12 +34,7 @@ export function getExtensionPointTargetSurface(extensionPointTarget: string) {
 }

 // Covers Customer Accounts UI extensions (future)
- case 'customeraccount':


This is from the legacy customer account targets, which are not used anymore

alxclark

lgtm!

jrrafols

The change looks good to me in general, I was just wondering about the sequencing of how this and related PRs will be shipped.

Should corresponding core PRs be built and shipped first to handle this change of how targets are handled at the CLI-level?

brianshen1990 · 2024-09-03T19:23:51Z

just as @jrrafols said, do we any plans to update related docs as well? one example https://shopify.dev/docs/apps/build/customer-accounts/pre-auth-order-status-page-extensions/build-pre-auth-order-status-page-extensions?extension=react#preview-the-extension (the 2nd preview the extension)

lihaokx · 2024-09-03T19:29:19Z

Should corresponding core PRs be built and shipped first to handle this change of how targets are handled at the CLI-level?

In my opinion, the sequencing of this pr and core prs does not matter too much. Devs will be in order index page after this change, which will make the behaviour of the 8 order-status targets consistent. I prefer to ship this one before the core prs, because cli is version-controlled and devs could be on legacy version or new version. We need to add the change to the earlier version ASAP.
@jrrafols

lihaokx · 2024-09-03T19:37:37Z

do we any plans to update related docs as well?

This is a good point. Since cli is version-controlled, devs may still see legacy behaviour if they are not on the latest version. I think we can change the doc after the stable version cli is released. @brianshen1990

jrrafols

I prefer to ship this one before the core prs, because cli is version-controlled and devs could be on legacy version or new version. We need to add the change to the earlier version ASAP.

Good point on versioning. I took a look and the next release is slated for Sept 16. I think it's reasonable to expect that the core changes will be available by then so I'm comfortable with this PR going out in the meantime.

As mentioned by @brianshen1990, we should also aim to have our documentation updated at around the same timeframe.

vividviolet

The changes look fine but is it possible to also make Checkout redirect to customer accounts for these targets? We can't really fix older releases of the CLI so Checkout should account for handling the legacy behaviour.

lihaokx · 2024-09-04T12:49:11Z

The changes look fine but is it possible to also make Checkout redirect to customer accounts for these targets?
We can't really fix older releases of the CLI so Checkout should account for handling the legacy behaviour.

If I understand correctly, you mean: in older version, does checkout redirect to customer account for the targets?

If so, the answer is yes, devs could be redirected to customer account after completing checkout in older version.

Here is the difference between older and newer version

Older version:

Clicking targets on cli console --> checkout --> completing checkout --> customer account

Newer version:

Clicking targets on cli console --> customer account

The main difference is that: in newer version, devs could be in customer account without going to checkout.

@vividviolet

vividviolet · 2024-09-04T17:40:34Z

The changes look fine but is it possible to also make Checkout redirect to customer accounts for these targets?
We can't really fix older releases of the CLI so Checkout should account for handling the legacy behaviour.

If I understand correctly, you mean: in older version, does checkout redirect to customer account for the targets?

If so, the answer is yes, devs could be redirected to customer account after completing checkout in older version.

Here is the difference between older and newer version

Older version:

Clicking targets on cli console --> checkout --> completing checkout --> customer account

Newer version:

Clicking targets on cli console --> customer account

The main difference is that: in newer version, devs could be in customer account without going to checkout.

@vividviolet

Got it. That makes sense 👍

order status targets as customer account targets

e34f232

lihaokx marked this pull request as draft September 3, 2024 14:31

lihaokx commented Sep 3, 2024

View reviewed changes

lihaokx marked this pull request as ready for review September 3, 2024 19:04

lihaokx requested review from jrrafols, vividviolet, alxclark, robin-drexler and brianshen1990 September 3, 2024 19:04

lihaokx changed the title ~~Set order status targets as customer account targets instead of checkout targets~~ Redirect order-status targets to customer account instead of checkout targets Sep 3, 2024

alxclark approved these changes Sep 3, 2024

View reviewed changes

jrrafols reviewed Sep 3, 2024

View reviewed changes

lihaokx requested a review from jrrafols September 3, 2024 20:00

jrrafols approved these changes Sep 3, 2024

View reviewed changes

brianshen1990 approved these changes Sep 3, 2024

View reviewed changes

vividviolet reviewed Sep 4, 2024

View reviewed changes

lihaokx requested a review from vividviolet September 4, 2024 13:19

vividviolet approved these changes Sep 4, 2024

View reviewed changes

lihaokx added this pull request to the merge queue Sep 4, 2024

Merged via the queue into main with commit 93fc48c Sep 4, 2024
38 checks passed

lihaokx deleted the customer-account-order-status branch September 4, 2024 17:57

shopify-shipit bot temporarily deployed to nightly September 4, 2024 17:57 Inactive

shopify-shipit bot temporarily deployed to experimental September 6, 2024 09:57 Inactive

lihaokx self-assigned this Sep 6, 2024

lihaokx added the #gsd:42127 Customer Account UI Extensibility GA label Sep 6, 2024

shopify-shipit bot temporarily deployed to production September 18, 2024 08:42 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redirect order-status targets to customer account instead of checkout targets #4410

Redirect order-status targets to customer account instead of checkout targets #4410

lihaokx commented Sep 3, 2024 •

edited

Loading

github-actions bot commented Sep 3, 2024

github-actions bot commented Sep 3, 2024

lihaokx Sep 3, 2024

alxclark left a comment

jrrafols left a comment

brianshen1990 commented Sep 3, 2024

lihaokx commented Sep 3, 2024 •

edited

Loading

lihaokx commented Sep 3, 2024

jrrafols left a comment

vividviolet left a comment

lihaokx commented Sep 4, 2024

vividviolet commented Sep 4, 2024

Older version:

Newer version:

Redirect order-status targets to customer account instead of checkout targets #4410

Redirect order-status targets to customer account instead of checkout targets #4410

Conversation

lihaokx commented Sep 3, 2024 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

Tophat

Measuring impact

Checklist

github-actions bot commented Sep 3, 2024

github-actions bot commented Sep 3, 2024

Coverage report

Test suite run success

lihaokx Sep 3, 2024

Choose a reason for hiding this comment

alxclark left a comment

Choose a reason for hiding this comment

jrrafols left a comment

Choose a reason for hiding this comment

brianshen1990 commented Sep 3, 2024

lihaokx commented Sep 3, 2024 • edited Loading

lihaokx commented Sep 3, 2024

jrrafols left a comment

Choose a reason for hiding this comment

vividviolet left a comment

Choose a reason for hiding this comment

lihaokx commented Sep 4, 2024

Older version:

Newer version:

vividviolet commented Sep 4, 2024

Older version:

Newer version:

lihaokx commented Sep 3, 2024 •

edited

Loading

lihaokx commented Sep 3, 2024 •

edited

Loading